Skip to content

feat(authz): expand permissions view#125

Merged
jacobo-dominguez-wgu merged 26 commits intoopenedx:masterfrom
WGU-Open-edX:feature/expanded-permissions-view
Apr 21, 2026
Merged

feat(authz): expand permissions view#125
jacobo-dominguez-wgu merged 26 commits intoopenedx:masterfrom
WGU-Open-edX:feature/expanded-permissions-view

Conversation

@jesusbalderramawgu
Copy link
Copy Markdown
Contributor

@jesusbalderramawgu jesusbalderramawgu commented Mar 31, 2026

Description

this PR closes this task
this needs to be reviewed once Jacobo's PR is merged
includes code from

Screenshot 2026-03-31 at 9 34 19 a m

Update this PR has been merged

@jesusbalderramawgu jesusbalderramawgu marked this pull request as draft March 31, 2026 15:35
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 31, 2026
@openedx-webhooks
Copy link
Copy Markdown

openedx-webhooks commented Mar 31, 2026

Thanks for the pull request, @jesusbalderramawgu!

This repository is currently maintained by @openedx/committers-frontend.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Mar 31, 2026
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Apr 1, 2026
@mphilbrick211 mphilbrick211 added the mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). label Apr 1, 2026
@jesusbalderramawgu jesusbalderramawgu force-pushed the feature/expanded-permissions-view branch 2 times, most recently from 7708f60 to 979521b Compare April 19, 2026 19:07
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (f1371b8) to head (f2839a1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   96.13%   96.20%   +0.07%     
==========================================
  Files          76       81       +5     
  Lines        1783     1869      +86     
  Branches      351      418      +67     
==========================================
+ Hits         1714     1798      +84     
- Misses         67       68       +1     
- Partials        2        3       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jesusbalderramawgu jesusbalderramawgu requested a review from dcoa April 19, 2026 21:48
@jesusbalderramawgu jesusbalderramawgu marked this pull request as ready for review April 19, 2026 21:50
@jesusbalderramawgu
Copy link
Copy Markdown
Contributor Author

I rebased this branch with the latest, made a minor fixes and added the proper tests.
is ready for review, could you please take a look @dcoa

@rodmgwgu
Copy link
Copy Markdown

I see some diferencies with the Figma design related to Global Staff, roles, I see that in the design, these roles are highlighted in gray, and show "Partial Access" under Permissions instead of "0 permissions available" as it's now.

Also, there seems to be some extra space at the bottom of the message for these cases that shouldn't be there.

Figma:
Screenshot 2026-04-20 at 1 02 16 p m

Implementation:
Screenshot 2026-04-20 at 1 04 47 p m


type CellProps = ExpandableTableRow<UserRole>;

export const ViewAllPermissionsCell = ({ row }: CellProps) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the components on this file to src/authz-module/components/TableCells.tsx to have all the custom table cells in the same place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@@ -0,0 +1,24 @@
import { useIntl } from '@edx/frontend-platform/i18n';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move all the subcomponents created inside src/authz-module/audit-user folder into src/authz-module/audit-user/components/ so it looks more organized?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I have updated this also I moved the components from the CustomCells component to TableCells component

@@ -1,5 +1,5 @@
import { renderHook, act } from '@testing-library/react';
import { QuerySettings } from '@src/authz-module/data/api';
import { QuerySettings } from '../data/api';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was better to use the absolute path for the import.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@@ -1,5 +1,5 @@
import { useCallback, useState } from 'react';
import { QuerySettings } from '@src/authz-module/data/api';
import { QuerySettings } from '../data/api';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@jesusbalderramawgu
Copy link
Copy Markdown
Contributor Author

I see some diferencies with the Figma design related to Global Staff, roles, I see that in the design, these roles are highlighted in gray, and show "Partial Access" under Permissions instead of "0 permissions available" as it's now.

Also, there seems to be some extra space at the bottom of the message for these cases that shouldn't be there.

Figma: Screenshot 2026-04-20 at 1 02 16 p m

Implementation: Screenshot 2026-04-20 at 1 04 47 p m

thank you @rodmgwgu this has been fixed, the background stopped working due to another PR
Screenshot 2026-04-20 at 2 39 43 p m

Copy link
Copy Markdown

@rodmgwgu rodmgwgu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this work, I requested some changes on details I found.

Comment thread src/authz-module/audit-user/index.tsx Outdated
import { useQuerySettings } from '@src/authz-module/hooks/useQuerySettings';
import { useUserAssignedRoles } from '@src/authz-module/data/hooks';
import messages from './messages';
import UserPermissions from '../components/UserPermissions';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use absolute imports here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

libraryPermissions,
rolesLibraryObject,
} from '../libraries/constants';
import RenderPermissionColumn from '../components/RenderPermissionColumn';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same in these, I'm not sure what's the project standard.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

@@ -0,0 +1,22 @@
import { useIntl } from '@edx/frontend-platform/i18n';
import messages from '../audit-user/messages';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

@@ -0,0 +1,95 @@
import { DJANGO_MANAGED_ROLES } from '@src/authz-module/constants';
Copy link
Copy Markdown

@rodmgwgu rodmgwgu Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this file (UserPermissions.tsx) also exists under components, and this one seems to not be used, please remove if not needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

Comment thread src/authz-module/audit-user/utils.ts Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is empty, do we need it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not anymore I have removed it

const expanded = (instance as any)?.state?.expanded || {};
Object.keys(expanded).forEach(rowId => {
if (rowId !== row.id && expanded[rowId]) {
(instance as any).toggleRowExpanded?.(rowId, false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid using any here? this may silently break in the future if the Paragon DataTable changes in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! I made the change!

Comment thread src/authz-module/audit-user/messages.ts Outdated
description: 'Description for the permissions of the Super Admin role',
},
'authz.user.table.permissions.role.staff': {
id: 'authz.user.table.permissions.role.staff ',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a trailing whitespace here, I guess it shouldn't be here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

Comment thread src/authz-module/libraries/constants.ts Outdated
},
];

export const DEFAULT_TOAST_DELAY = 5000;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These constants seem to be repeated in libraries/constants and in src/authz-module/constants.ts, let's have them in just one place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been fixed!

Comment thread src/data/hooks.ts
queryFn: async () => getUserAccount(username),
retry: false,
enabled: !!username,
refetchOnWindowFocus: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change intentional? usually we don't want to refetch user information on every focus.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

Copy link
Copy Markdown
Contributor

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as expected, and I see the code fine! I mainly point to the texts, but I think they are going to be received translated from the backend in the future, aren't they?

Additionally, I think it would be cool if the cursor were pointer on ViewMoreLink hover, and avoid the text 0 permissions available when the user has Global scope permissions, as it is contradictory.

Isn't this going to be added by now?

Image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw there is similar data at src/frontend-app-admin-console/src/authz-module/roles-permissions/courses/constants.ts

are we going to delete that file in favor of this?

Comment thread src/authz-module/courses/constant.ts Outdated
{
key: CONTENT_COURSE_PERMISSIONS.EDIT_COURSE_FILES,
resource: 'course_files',
description: 'Permanently remove files and assets from the course.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: 'Permanently remove files and assets from the course.',
description: 'Perform non-destructive actions on files, such as locking or unlocking them.',

key: CONTENT_LIBRARY_PERMISSIONS.EDIT_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Publish', description: 'Update the name and contents of existing collections.', icon: EditOutline,
},
{
key: CONTENT_LIBRARY_PERMISSIONS.DELETE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Edit', description: 'Permanently remove collections from the library.', icon: EditOutline,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key: CONTENT_LIBRARY_PERMISSIONS.DELETE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Edit', description: 'Permanently remove collections from the library.', icon: EditOutline,
key: CONTENT_LIBRARY_PERMISSIONS.DELETE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Delete', description: 'Permanently remove collections from the library.', icon: Delete,

Comment thread src/authz-module/courses/constant.ts Outdated
{
key: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_CERTIFICATES,
resource: 'course_advanced_certificates',
description: 'Access and edit the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: 'Access and edit the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.',
description: 'Create and edit course certificates, including certificate design and eligibility settings.',

Comment thread src/authz-module/courses/constant.ts Outdated
{
key: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_DETAILS,
resource: 'course_schedule_details',
description: 'See course information including the course summary, pacing, and prerequisites..',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: 'See course information including the course summary, pacing, and prerequisites..',
description: 'See course information including the course summary, pacing, and prerequisites.',

{
key: CONTENT_COURSE_PERMISSIONS.DELETE_COURSE_FILES,
resource: 'course_files',
description: 'Delete files.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: 'Delete files.',
description: 'Permanently remove files and assets from the course.',

key: CONTENT_LIBRARY_PERMISSIONS.MANAGE_LIBRARY_TEAM, resource: 'library_team', label: 'Manage', description: 'Add, change, or remove role assignments for this library from the Roles and Permissions console.', icon: Settings,
},
{
key: CONTENT_LIBRARY_PERMISSIONS.CREATE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'View', description: 'Create new collections to organize content within the library.', icon: RemoveRedEye,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key: CONTENT_LIBRARY_PERMISSIONS.CREATE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'View', description: 'Create new collections to organize content within the library.', icon: RemoveRedEye,
key: CONTENT_LIBRARY_PERMISSIONS.CREATE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Create', description: 'Create new collections to organize content within the library.', icon: RemoveRedEye,

key: CONTENT_LIBRARY_PERMISSIONS.CREATE_LIBRARY_COLLECTION, resource: 'library_collection', label: 'View', description: 'Create new collections to organize content within the library.', icon: RemoveRedEye,
},
{
key: CONTENT_LIBRARY_PERMISSIONS.EDIT_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Publish', description: 'Update the name and contents of existing collections.', icon: EditOutline,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key: CONTENT_LIBRARY_PERMISSIONS.EDIT_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Publish', description: 'Update the name and contents of existing collections.', icon: EditOutline,
key: CONTENT_LIBRARY_PERMISSIONS.EDIT_LIBRARY_COLLECTION, resource: 'library_collection', label: 'Edit', description: 'Update the name and contents of existing collections.', icon: EditOutline,

Comment thread src/authz-module/courses/constant.ts Outdated
{
key: CONTENT_COURSE_PERMISSIONS.MANAGE_COURSE_ADVANCED_SETTINGS,
resource: 'course_advanced_certificates',
description: 'Access and edit the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options..',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: 'Access and edit the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options..',
description: 'Access and edit the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.',

Comment thread src/index.scss Outdated
@import "~@edx/frontend-component-header/dist/index";
@use "@openedx/paragon/dist/core.min.css";
@use "@openedx/paragon/dist/light.min.css";
@import "~@edx/frontend-component-header/dist/index"; No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@import "~@edx/frontend-component-header/dist/index";
@import "~@edx/frontend-component-header/dist/index";

@jesusbalderramawgu
Copy link
Copy Markdown
Contributor Author

It works as expected, and I see the code fine! I mainly point to the texts, but I think they are going to be received translated from the backend in the future, aren't they?

Additionally, I think it would be cool if the cursor were pointer on ViewMoreLink hover, and avoid the text 0 permissions available when the user has Global scope permissions, as it is contradictory.

Isn't this going to be added by now?

Image

Thank you @bra-i-am I have addressed your comments, I just didn't change the texts because the ones that I have right now were specified in the task

Copy link
Copy Markdown
Contributor

@jacobo-dominguez-wgu jacobo-dominguez-wgu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and works as expected!

@jacobo-dominguez-wgu
Copy link
Copy Markdown
Contributor

@bra-i-am, @rodmgwgu do you wanna make another review before merging?

Copy link
Copy Markdown

@rodmgwgu rodmgwgu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jesusbalderramawgu jesusbalderramawgu force-pushed the feature/expanded-permissions-view branch from 16022e5 to 0f4610a Compare April 21, 2026 16:29
@jacobo-dominguez-wgu jacobo-dominguez-wgu merged commit 0d4f93e into openedx:master Apr 21, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Waiting on Author to Done in Contributions Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mao-onboarding Reviewing this will help onboard devs from an Axim mission-aligned organization (MAO). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants